fix(sports): stop rendering event start times in UTC (baseball + 9 plugins) - #228
Merged
Merged
Conversation
The plugin looked for the LEDMatrix global timezone only on cache_manager.config_manager. On cores that expose config_manager via the plugin manager instead, that lookup came back empty and every start time was drawn in UTC -- a 6:45pm Central first pitch rendered as 11:45PM -- while clock-simple and geochron, which check plugin_manager first, showed the correct local time on the same device. Timezone resolution now lives in one place (baseball_timezone.py) shared by the switch-mode scorebug, the scroll-mode game card and the plugin manager. Order: plugin override, plugin_manager.config_manager, cache_manager.config_manager, host system zone (TZ, /etc/timezone, /etc/localtime), then UTC. Two related fixes make an explicit override actually usable: - config_schema.json never declared `timezone` and sets additionalProperties: false, so hand-editing the key in the saved config had no effect. It is now a documented string property under Advanced Settings. - The manager assigned self.config["timezone"], mutating the dict the core handed it and persisting a bogus "timezone": "UTC" into the user's saved plugin config. The resolved value is kept on the instance and passed to sub-components via a copy instead. Adds test_timezone_resolution.py covering the resolution order, invalid and blank values, a raising config_manager, and the system-zone backstop. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QSnspNZceRCdpUtJh2Co6e
Contributor
|
Warning Review limit reached
Next review available in: 55 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (68)
📝 WalkthroughWalkthroughBaseball Scoreboard 1.20.0 centralizes timezone resolution, adds an advanced timezone configuration property, updates manager and rendering paths to use resolved timezones without mutating saved configuration, adds regression tests, and documents the release. ChangesBaseball Scoreboard timezone handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Manager
participant TimezoneResolver
participant ConfigManagers
participant Renderer
Manager->>TimezoneResolver: Resolve effective timezone
TimezoneResolver->>ConfigManagers: Check plugin and core configuration
ConfigManagers-->>TimezoneResolver: Return candidate timezone
TimezoneResolver-->>Manager: Return validated timezone
Manager->>Renderer: Pass resolved timezone configuration
Renderer->>TimezoneResolver: Resolve timezone for game start
TimezoneResolver-->>Renderer: Return tzinfo
Renderer->>Renderer: Convert UTC start time to local time
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 468 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/baseball-scoreboard/baseball_timezone.py`:
- Around line 107-118: Update the timezone candidate resolution around the
candidates collection and its consuming loop to evaluate sources lazily,
yielding each candidate only when reached. Preserve the existing priority
order—plugin config, plugin-manager config, cache-manager config, then system
timezone—and stop evaluating lower-priority sources once a valid timezone is
found.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7a6255e8-da3a-4f12-b821-8e77eee6962a
📒 Files selected for processing (10)
plugins.jsonplugins/baseball-scoreboard/CHANGELOG.mdplugins/baseball-scoreboard/README.mdplugins/baseball-scoreboard/baseball_timezone.pyplugins/baseball-scoreboard/config_schema.jsonplugins/baseball-scoreboard/game_renderer.pyplugins/baseball-scoreboard/manager.pyplugins/baseball-scoreboard/manifest.jsonplugins/baseball-scoreboard/sports.pyplugins/baseball-scoreboard/test_timezone_resolution.py
resolve_timezone_name() built its candidate list eagerly, so every call queried both config managers and stat'd the host timezone files even when the plugin config already supplied a valid zone. SportsCore._get_timezone() runs once per game during extraction, making that a per-game filesystem probe. Yield candidates from a generator instead, preserving the priority order, and assert in the tests that lower-priority sources are left untouched once a candidate resolves. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QSnspNZceRCdpUtJh2Co6e
Ports the baseball-scoreboard timezone fix to every other plugin carrying the same defect. All nine looked for the LEDMatrix global timezone only on cache_manager.config_manager; on cores that expose config_manager via the plugin manager instead, that lookup came back empty and every start time was drawn in UTC, while clock-simple and geochron -- which check plugin_manager first -- showed the correct local time on the same device. Each plugin gets its own plugin-prefixed resolver module (football_timezone.py, hockey_timezone.py, ...) rather than a shared one, since the core loads plugin top-level modules as bare names. Resolution order: plugin override, plugin_manager.config_manager, cache_manager.config_manager, host system zone (TZ, /etc/timezone, /etc/localtime), then UTC. Candidates are evaluated lazily so the per-event path stops at the first valid source. Also, in every plugin: - config_schema.json never declared `timezone`, and seven of the nine set additionalProperties: false, so hand-editing the key was discarded. It is now a documented string property under Advanced Settings, with x-propertyOrder kept in sync where the schema defines one. - football-scoreboard additionally assigned self.config["timezone"] in on_config_change, mutating the core's dict and persisting a stale "timezone": "UTC" that then shadowed the real global timezone. Removed. Versions bumped (minor -- new config property): football 2.9.0, hockey 1.5.0, basketball 1.8.0, soccer 2.4.0, ufc 1.3.0, lacrosse 1.5.0, nrl 1.1.0, afl 1.1.0, f1 1.6.0. Each plugin gets test_timezone_resolution.py (11 cases) covering the resolution order, lazy evaluation, invalid and blank values, a raising config_manager, and the system-zone backstop. Module-collision check passes; the pre-existing test failures in these plugins are unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QSnspNZceRCdpUtJh2Co6e
ChuckBuilds
changed the title
… name
The safety harness went red on ufc-scoreboard with
ModuleNotFoundError("No module named 'logo_downloader'") at plugin load.
Pre-existing, not introduced here: sports.py imported the bare name
`logo_downloader`, but ufc-scoreboard ships no such module. The core loads
plugin top-level .py files as bare-name modules on sys.path, so the import only
ever succeeded by binding a *different* plugin's logo_downloader -- exactly the
cross-plugin coupling scripts/check_module_collisions.py exists to prevent --
and raised outright when no such plugin happened to be loaded. The harness only
runs *changed* plugins, so it had never exercised ufc-scoreboard before this
branch touched it.
Now imports src.logo_downloader from the core, matching the soccer, nrl and afl
plugins and this same file's existing deferred `from src.logo_downloader import
LogoDownloader` a few lines below, including their sys.path preamble.
Verified sports.py and mma.py import cleanly against a stub src.logo_downloader
and that LogoDownloader now resolves to src.logo_downloader. Folded into the
unreleased 1.3.0 notes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QSnspNZceRCdpUtJh2Co6e
ChuckBuilds
pushed a commit
that referenced
this pull request
Jul 30, 2026
…mes release Both branches bumped soccer-scoreboard to 2.4.0 — main's is the UTC start-time fix (#228). Rebased this work onto 2.5.0 so main's release notes are preserved beneath it rather than replaced. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
ChuckBuilds
pushed a commit
that referenced
this pull request
Jul 30, 2026
…-times release #228 landed and also bumped baseball-scoreboard, so both branches claimed 1.20.0. This work is now 1.20.1 with main's release notes preserved beneath it rather than replaced. manager.py and config_schema.json auto-merged; verified the merged manager.py compiles and carries both changes (baseball_timezone from #228 and the Vegas slate work here). Harness passes all 24 screens post-merge. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
This was referenced Jul 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Summary
Every sports scoreboard rendered event start times in UTC (a 6:45pm Central first pitch showed as
11:45PM) even on setups where the clock plugin displayed the correct local time. All ten looked for the LEDMatrix global timezone only oncache_manager.config_manager; on cores that exposeconfig_managervia the plugin manager instead, that lookup came back empty and resolution fell through to UTC. This centralizes timezone resolution per plugin, adds the missing fallbacks, and makes thetimezonesetting an actual, documented config option.Originally scoped to
baseball-scoreboard; extended after an audit found the identical defect in nine more plugins.Type of change
Plugin(s) affected
Related issues
N/A — reported directly.
What changed
A plugin-prefixed resolver module per plugin (
baseball_timezone.py,football_timezone.py, …) — separate copies rather than one shared module, because the core loads plugin top-level modules as bare names and namespace-isolates them afterward. Each is the single source of truth for its plugin's render paths and manager. First valid value wins:timezonein the plugin's own config (explicit override)plugin_manager.config_manager— the lookup that was missing everywhere;clock-simpleandgeochroncheck it first, which is why they were correct on the same devicecache_manager.config_manager(what these plugins checked exclusively)TZ,/etc/timezone,/etc/localtimeCandidates are evaluated lazily, so the per-event path stops at the first valid source instead of probing the filesystem on every render. Also supports older cores exposing
load_config()/get_config()rather thanget_timezone(), skips blank/invalid zone names with a warning, and never propagates an exception out of a core config manager.Two related fixes make an explicit override actually usable:
config_schema.jsonnever declaredtimezone, and seven of the ten setadditionalProperties: false— so hand-editing the key in the saved config was discarded. It is now a string property under Advanced Settings, defaulting to"", withx-propertyOrderkept in sync where the schema defines one.baseballandfootballmutated the config dict the core handed them (self.config["timezone"] = ...), persisting a bogus"timezone": "UTC"into the user's saved config which then shadowed the real global timezone. Both now keep the resolved value on the instance.Left alone deliberately:
march-madness(hardcodesUS/Easternfor tip-off times),olympics(already declares its owntimezone),ledmatrix-weather(uses the forecast location's zone, not the global config), andclock-simple/geochron/7-segment-clock/calendar/odds-ticker(already checkplugin_managerfirst).Test plan
EMULATOR=true python3 run.py)scripts/dev_server.py)test_timezone_resolution.pyin each of the ten plugins (11 cases each, 110 total, all passing): resolution order, the plugin-level override winning, theplugin_managerlookup that was missing, lazy evaluation (asserts lower-priority sources go untouched once a candidate resolves), the legacyload_config()shape, aget_timezone()that raises, blank/invalid values being skipped, the system-zone backstop, the UTC last resort, and an end-to-end conversion of the reported23:45Z → 6:45PMcase.Also run:
python scripts/check_module_collisions.py— OK across 41 pluginstest_*.pyin all ten plugins, before and after. The seven that fail (football ×3, hockey ×2, basketball ×2, plus four in baseball) fail identically onmain— they need the coresrcpackage and font assets not present in this environment. No regressions; also not a useful signal.config_schema.json/manifest.jsonre-parsed as JSON; schema diffs are purely additive.Not verified on hardware or in the emulator — worth a sanity check on a real panel before release.
Required for plugin changes
versioninplugins/<id>/manifest.jsonfor all ten, each with aversions[]changelog entryclass_nameinmanifest.jsonmatches the actual class inmanager.pyexactly (unchanged)entry_pointmatches the real file (unchanged)README.mdif config keys changed — all ten documenttimezone; those with a Troubleshooting section also get an entry for the UTC symptomconfig_schema.jsonis the source of truth for the web UI form —timezoneadded withdefault,descriptionandx-advancedplugins.json)Checklist
CONTRIBUTING.mdCONTRIBUTING.mdandCODE_OF_CONDUCT.mdNotes for reviewer
Existing installs of baseball and football need one manual step. Anyone who ran an affected version has
"timezone": "UTC"written into their saved config by the old write-back bug. Since an explicit plugin-level value wins, that stale"UTC"keeps overriding the fix until it's cleared or corrected. It's now editable in the web UI under Advanced Settings, and the READMEs call it out. I deliberately avoided auto-healing a stored"UTC"— there's no way to distinguish it from a deliberate choice, and silently overriding it would break users who genuinely want UTC.Why nine copies of the same module rather than one shared file: the loader's bare-name import plus post-load namespace isolation makes a shared name risky for anything imported from a deferred or subpackage position, and
scripts/check_module_collisions.pyenforces plugin-unique names for exactly this reason. The duplication is deliberate and matches how these plugins already ship their ownsports.py,scroll_display.py, etc.The mechanical edits (resolver module, schema property, version bump, test) were script-generated from the baseball original and then verified per plugin; the non-uniform bits — hockey's second
_get_timezoneinbase_classes.py, soccer's two manager call sites, f1's different_resolve_timezonesignature, football's write-back — were handled individually.